Defer consensus verification #2350
Conversation
| // If the set of preferred IDs already contains the preference, then the | ||
| // tail is guaranteed to already be set correctly. This is because the value | ||
| // returned from vote reports the next preferred block after the last | ||
| // preferred block that was voted for. If this block was previously | ||
| // preferred, then we know that following the preferences down the chain | ||
| // will return the current tail. | ||
| if ts.preferredIDs.Contains(preferred) { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
This PR removes this optimization. Should look into how important it was.
There was a problem hiding this comment.
Is this because it is incompatible with these changes or for some other reason?
There was a problem hiding this comment.
It wasn't really compatible - because we may still want to update our preference due to something passing verification that previously failed... When we change the code to evict invalid blocks - I think this optimization makes sense again.
| parent, err := b.vm.getPreForkBlock(ctx, b.Block.Parent()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return parent.verifyPreForkChild(ctx, b) | ||
| } | ||
|
|
||
| func (b *preForkBlock) Verify(ctx context.Context) error { | ||
| return nil // Block verification is fully handled by VerifyProposer |
| @@ -866,6 +866,13 @@ func (b *blockClient) Parent() ids.ID { | |||
| return b.parentID | |||
| } | |||
|
|
|||
| func (b *blockClient) VerifyProposer(ctx context.Context) error { | |||
| _, err := b.vm.client.BlockVerifyProposer(ctx, &vmpb.BlockVerifyProposerRequest{ | |||
There was a problem hiding this comment.
Why is this required if the VM always returns nil? ava-labs/coreth@2486845
Is this an optimistic future where the VM would manage proposer validity?
vms/proposervm/post_fork_block.go
Outdated
| } | ||
|
|
||
| func (b *postForkBlock) verifyPostForkChild(ctx context.Context, child *postForkBlock) error { | ||
| parentPChainHeight := b.PChainHeight() |
There was a problem hiding this comment.
nit: variable declaration not needed here?
|
This PR has become stale because it has been open for 30 days with no activity. Adding the |
|
This PR has become stale because it has been open for 30 days with no activity. Adding the |
Why this should be merged
How this works
How this was tested